Skip to content

ref(node): Streamline lru-memoizer instrumentation#21350

Merged
nicohrubec merged 2 commits into
developfrom
nh/streamline-lrumemoizer
Jun 8, 2026
Merged

ref(node): Streamline lru-memoizer instrumentation#21350
nicohrubec merged 2 commits into
developfrom
nh/streamline-lrumemoizer

Conversation

@nicohrubec

@nicohrubec nicohrubec commented Jun 5, 2026

Copy link
Copy Markdown
Member

Streamlines the vendored @opentelemetry/instrumentation-lru-memoizer:

  • Ported the upstream OTel unit tests for the instrumentation but using a fake lru memoizer instead of real module.
  • Removed the unused config constructor param (the SDK always constructs it with no config).

Fixes #20735

@nicohrubec nicohrubec force-pushed the nh/streamline-lrumemoizer branch 3 times, most recently from edab9dc to 2a2df78 Compare June 8, 2026 08:25
Vendor the upstream OTel unit tests and clean out the dead config passthrough.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nicohrubec nicohrubec force-pushed the nh/streamline-lrumemoizer branch from 2a2df78 to 811b01c Compare June 8, 2026 08:41
@nicohrubec nicohrubec marked this pull request as ready for review June 8, 2026 08:43
@nicohrubec nicohrubec requested a review from a team as a code owner June 8, 2026 08:43
@nicohrubec nicohrubec requested review from JPeer264 and andreiborza and removed request for a team June 8, 2026 08:43
@nicohrubec nicohrubec changed the title test(node): Streamline lru-memoizer instrumentation ref(node): Streamline lru-memoizer instrumentation Jun 8, 2026
@@ -0,0 +1,112 @@
/*

@nicohrubec nicohrubec Jun 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mhm on second thought: since we are gonna rewrite the instrumentation to orchestrion soon, maybe porting the unit tests might actually be too close to the implementation to make sense? as in we probably will have to rewrite these then anyways when porting to orchestrion?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests themselves test instrumentation details right? I think it's fine, we'll only have to swap out the patching mechanism I guess, unless I'm missing something.

@andreiborza andreiborza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nicohrubec nicohrubec merged commit d645349 into develop Jun 8, 2026
174 of 175 checks passed
@nicohrubec nicohrubec deleted the nh/streamline-lrumemoizer branch June 8, 2026 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Streamline @opentelemetry/instrumentation-lru-memoizer

2 participants